Skip to content

Fix Redis driver pipeline parsing and runtime/server test initialization#173

Merged
hotlong merged 3 commits intomainfrom
copilot/fix-action-run-issue-again
Jan 23, 2026
Merged

Fix Redis driver pipeline parsing and runtime/server test initialization#173
hotlong merged 3 commits intomainfrom
copilot/fix-action-run-issue-again

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jan 23, 2026 •
edited
Loading

Fixes two issues preventing CI from passing:

1. Redis Driver Pipeline Result Parsing

The Redis driver bulkUpdate and bulkDelete operations were parsing pipeline results using the v3 client API format [error, result], but the project uses v4 which returns results directly.

bulkUpdate:

  • Access pipeline GET results directly instead of via tuple indexing: getResults?.[i] vs result?.[1]
  • Guard against empty pipeline execution when no records exist

bulkDelete:

  • Filter DEL results directly: r > 0 vs r && r[1] > 0

Example

// Before (v3 API assumption)
const result = getResults?.[i] as any;
const existingData = result?.[1];  // Assumes [error, result] tuple

// After (v4 API)
const existingData = getResults?.[i];  // Direct result access

The v4 client returns [1, 1, 0] for three DEL operations (two successful, one non-existent key), not [[null, 1], [null, 1], [null, 0]].

2. Runtime/Server Test Initialization

Added missing await app.init() calls to test setup in 4 test files that were failing because ObjectQL v4.0 requires initialization before use:

  • node.test.ts
  • rest.test.ts
  • graphql.test.ts
  • rest-advanced.test.ts

These failures were previously hidden by pnpm's "stop on first failure" behavior until the Redis driver tests were fixed.

Original prompt

引用: https://github.com/objectstack-ai/objectql/actions/runs/21284280204/job/61261497154#step:10:1

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

@vercel
Copy link
Copy Markdown

vercel bot commented Jan 23, 2026 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
objectql-site Ready Ready Preview, Comment Jan 23, 2026 0:01am

Request Review

@hotlong
Fix Redis driver bulkUpdate and bulkDelete for Redis v4 client
0b8bc17 
- Fixed bulkUpdate to handle Redis v4 client pipeline results (direct values, not tuples)
- Fixed bulkDelete to parse Redis v4 pipeline results correctly
- Added guard to prevent executing empty pipeline in bulkUpdate
- Added explanatory comments about Redis v4 client behavior

Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix issue with action run in ObjectQL Fix Redis driver pipeline result parsing for v4 client Jan 23, 2026
Copilot AI requested a review from hotlong January 23, 2026 11:44
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 23, 2026

⚠️ No Changeset Found

This PR does not include a changeset file.
If this PR includes user-facing changes, please add a changeset by running:

pnpm changeset

@hotlong
Copy link
Copy Markdown
Contributor

hotlong commented Jan 23, 2026

@copilot fix test

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jan 23, 2026

@copilot fix test

The Redis driver tests that were originally failing are now fixed (commit 0b8bc17). However, the CI is now showing failures in packages/runtime/server tests that were previously hidden because pnpm stops at the first test failure.

These server test failures appear to be pre-existing issues from the v4.0 merge (#172), not caused by my Redis driver changes. Would you like me to investigate and fix the runtime/server test failures as well?

@hotlong
Fix runtime/server tests by adding missing app.init() calls
2f67c76 
Added await app.init() to test setup in:
- node.test.ts
- rest.test.ts
- graphql.test.ts
- rest-advanced.test.ts

These tests were failing because ObjectQL v4.0 requires initialization before use.

Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
Copilot AI changed the title Fix Redis driver pipeline result parsing for v4 client Fix Redis driver pipeline parsing and runtime/server test initialization Jan 23, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 23, 2026

⚠️ No Changeset Found

This PR does not include a changeset file.
If this PR includes user-facing changes, please add a changeset by running:

pnpm changeset

@hotlong hotlong marked this pull request as ready for review January 23, 2026 15:54
Copilot AI review requested due to automatic review settings January 23, 2026 15:54
@hotlong hotlong merged commit 6f483dd into main Jan 23, 2026
8 checks passed
@hotlong hotlong deleted the copilot/fix-action-run-issue-again branch January 23, 2026 15:54
Copilot AI reviewed Jan 23, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes Redis pipeline result parsing to align with the Redis v4 client API and ensures ObjectQL runtime/server tests correctly initialize the app before use so CI can pass.

Changes:

  • Updated RedisDriver.executeCommand bulk operations (bulkUpdate, bulkDelete) to treat multi().exec() results as direct replies instead of [error, result] tuples, and added a safety guard to avoid executing empty pipelines.
  • Added missing await app.init() calls in REST, REST advanced, GraphQL, and Node server tests so handlers are created only after the ObjectQL app is fully initialized.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/drivers/redis/src/index.ts Adjusts Redis pipeline result handling for bulkUpdate and bulkDelete to match the v4 client API and guards setPipeline.exec() when no updates are enqueued.
packages/runtime/server/test/rest.test.ts Ensures the REST adapter tests call await app.init() before creating the REST handler and HTTP server.
packages/runtime/server/test/rest-advanced.test.ts Ensures advanced REST/error-handling tests initialize the ObjectQL app before building the handler and server.
packages/runtime/server/test/node.test.ts Ensures Node adapter tests call await app.init() after schema registration so requests run against an initialized app.
packages/runtime/server/test/graphql.test.ts Ensures GraphQL adapter tests initialize the ObjectQL app before creating the GraphQL handler and server.
Comments suppressed due to low confidence (2)

packages/runtime/server/test/node.test.ts:13

  • Unused import IObjectQL.
import { IObjectQL, Driver } from '@objectql/types';

packages/drivers/redis/src/index.ts:336

  • This use of variable 'filters' always evaluates to true.
        if (filters && !Array.isArray(filters) && filters.filters) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@hotlong hotlong Awaiting requested review from hotlong

Assignees

@hotlong hotlong

Copilot code review Copilot

Labels

🔌 drivers 🚀 runtime size/S 🧪 tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hotlong